Conversation
✅ Deploy Preview for blogtorontojscom ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
This tripped me up during testing. All links went to the production URL. Makes more sense to make these relative I think!
|
What command were you using to test it? If you do use
|
src/layouts/BlogPosts.astro
Outdated
| <PostCard | ||
| variant="large" | ||
| url={`${BLOG_URL}${post.url}`} | ||
| url={post.url} |
There was a problem hiding this comment.
Could you check that post.url correctly resolves to an absolute url so we don't have inexistant relative urls?
There was a problem hiding this comment.
No it's a relative URL, and that's what you want here I think!
But it does resolve and made clicking through the site while testing a lot easier.
There was a problem hiding this comment.
My concern is that we have relative URLs like: ./2024/10/some-post and that will break some pages like the tags pages. For example, if you go to /tags/web-development and have the relative URL post, it will not work as expected.
There was a problem hiding this comment.
Oh no, the URLs start with a slash! So that should be good
There was a problem hiding this comment.
I just checked, the urls don't start with slashes... 😕
There was a problem hiding this comment.
Here is an example of an url on the tags page:
There was a problem hiding this comment.
Oof sorry, I must have looked at this wrong. Will fix this PR!
There was a problem hiding this comment.
Thank you for checking
There was a problem hiding this comment.
No problem, I just fixed the issue on the root. Take a look at this branch: relative-uris...fix/post-urls
It also fixes the issues with the rss feed (turns out it was missing a namespace and the reference to get the posts was wrong)
|
Fixed! And I also changed a few more instances of this issue |
There was a problem hiding this comment.
Let me be very pedantic here: I think it would be better if we fix this at the source, by making post.url start with a slash than fix in multiple places.
Another option would be to add a url formatting utility, but that may be more complex.
Aside from that it looks good to me, feel free to merge and we can make it more generic in a next PR.
|
I'm ok with pedantic! I assumed |
|
It is a built-in, but we are overriding it at the source to include the year and month. I made this change and updated some other places on the branch |
2 participants
This tripped me up during testing. All links went to the production URL, so it looked like the new page i was adding was 404ing, but I didn't realize I was no longer on the same domain.
Makes more sense to make these relative I think!